-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support CVAT 2.X deployment using helm #4448
Conversation
Previously those were erronously taken from backend.
Added templates for OPA and extended readme on how to deploy. Moreover modernized the ingress chart to support k8s version 1.22+.
@se-wo , thanks for the contribution! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@se-wo thank you for the your contribution,
Generally I'm OK with changes, but could you please fix Markdown linter warning and add minor note about archive?
Also could you please add a note to the changelog, I think it would be helpful to the community.
Can you give me a hint on what the linter issue is or what linter style is used? I am only able to see that the linter Github action failed. I fixed everything the default VS Code linter reported to me before opening this pull request. |
@azhavoro added a changelog entry. Updated the readme with information about where to create the rules.tar.gz. Also added the shell command that I used to create the archive. |
@@ -15,7 +15,7 @@ type: application | |||
# This is the chart version. This version number should be incremented each time you make changes | |||
# to the chart and its templates, including the app version. | |||
# Versions are expected to follow Semantic Versioning (https://semver.org/) | |||
version: 0.2.0 | |||
version: 0.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should match chart version with CVAT's release version?
Thanks for the effort! I deployed my chart from this branch, and it works like a charm 🎉 |
@se-wo LGTM, could you please fix linter warning, I've attached the report |
Fix linter error due to markdown line longer than 120 characters
@azhavoro Linter error should be fixed. Was only a single line that exceeded 120 characters. Have split it into multiple lines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@se-wo , Thanks for the contribution!
Motivation and context
The current charts provided in the helm-charts directory are broken. The latest docker images require that Open Policy Agent is available. OPA was added to the docker-compose files but not to the helm charts. This pull requests fixes this problem.
Moreover the templates were updated to support newer kubernetes versions. APIs like networking.k8s.io/v1beta1 and extensions/v1beta1 no longer exist in kubernetes versions 1.22 onwards. This pull request also adds support for networking.k8s.io/v1. Further, I fixed a small mistake where frontend templates used backend labels instead of their own.
Multiple issues like #4438 and #4192 are looking for a solution to CVAT 2.X support.
How has this been tested?
The helm charts were tested locally with minikube v1.25.2 running k8s v1.23.3. Charts were deployed using Helm 3.8
Checklist
develop
branchI have added tests to cover my changescvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.